TE with threading build #1092
Merged
TE with threading build #1092
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Phuong Nguyen <[email protected]>
timmoon10
reviewed
Aug 9, 2024
Signed-off-by: Phuong Nguyen <[email protected]>
Signed-off-by: Phuong Nguyen <[email protected]>
|
I updated the default value for threading to 1 and added a message for parallel build info. |
timmoon10
reviewed
Aug 10, 2024
Co-authored-by: Tim Moon <[email protected]> Signed-off-by: Phuong Nguyen <[email protected]>
Signed-off-by: Phuong Nguyen <[email protected]>
Signed-off-by: Phuong Nguyen <[email protected]>
ksivaman
reviewed
Aug 12, 2024
| @@ -68,7 +68,7 @@ def setup_pytorch_extension( | |||
| print("Could not determine CUDA Toolkit version") | |||
| else: | |||
| if version >= (11, 2): | |||
| nvcc_flags.extend(["--threads", "4"]) | |||
| nvcc_flags.extend(["--threads", os.getenv("NVTE_BUILD_THREADS_PER_JOB", "1")]) | |||
There was a problem hiding this comment.
@phu0ngng could you document here the reason for switching from 4 to 1 as a default?
There was a problem hiding this comment.
As @timmoon10 mentioned, users getting stuck in the build process (#976, #978, #1077).
Besides, the PyTorch Ext build with --thread 4 broke on my desktop. I discussed with Tim and we think it is better to have 1 as default so that all users have fewer build issues.
For "advanced" users, they are encouraged to play with NVTE_BUILD_THREADS_PER_JOB to best match their system.
mgoldfarb-nvidia
pushed a commit
to mgoldfarb-nvidia/TransformerEngine
that referenced
this pull request
Aug 14, 2024
* added threading build back * integrating threading for pytorch and paddle extensions * added messages --------- Signed-off-by: Phuong Nguyen <[email protected]> Co-authored-by: Tim Moon <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In #987, TE build with parallel jobs was added, while threading for fused_attn build was removed. With #1048, the build time of the ext can be measured and compared.
It turns out that using max parallel jobs together with setting
--threads 4halves the build time. However, it adds some potential oversubscribes that may break the build on NGC. SoNVTE_BUILD_THREADS_PER_JOBenv var was added to provide an option to disable this threading build.By default,
NVTE_BUILD_THREADS_PER_JOB=1to avoid potential build issues on small systems.Users are encouraged to play with the
NVTE_BUILD_THREADS_PER_JOBvalue based on their system.NVTE_MAX_BUILD_JOBSwas renamed toNVTE_BUILD_MAX_JOBSfor consistency.Type of change
Checklist: